perf(macOS): avoid copying large protocol bodies#1719
Conversation
Package Changes Through 6a3aa79There are 1 changes which include wry with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
There was a problem hiding this comment.
Don't think I understand this, NSData::from_vec uses initWithBytes_length internally as well, how is it avoiding the copy here? And why are we only doing this for the bigger chunks?
That being said, I don't mind if we migrate to NSData::with_bytes here for readability though
|
NSData::from_vec did not use The chunk size is a workaround based on local benchmarks; it yields benefits by eliminating buffer copying starting from 88 bytes. However, sizes below 88 bytes cause regressions. 128 bytes as it aligns better with engineering standards and is more human-readable. |
|
Sorry my bad, looked at the wrong line 🤦♂️, thanks for checking |
Legend-Master
left a comment
There was a problem hiding this comment.
Looks good to me, just some nitpicks
…pying for larger responses
|
Some custom benchmark, for only IPC performance part.
|
No, you good. I get some mess at time. :) |
I'm entirely sure I get the table headers, like is |
|
Adjusted my benchmark closer to IPC performance and avoid zero data optimize for benchmark, it tells 64Kb could get around 3.33%-4% improvement which conflict with the previous lower than 88KB leads regression, but now latest no-copy size regression(noise) happens on lower than 64KB. But 64KB performance have very slightly improved. Let's keep threshold to 128 for engineering standards. dev: wry 44e26ef Greater than 64KB do have significant improvement.
Lower than 64KB most are noise or regression at sometimes, not stable to test.
|
37cf53f to
226a006
Compare
|
If the regression is mostly noises, what about removing the threshold? |
Sort of, but really can't tells the real world performance. For stability let's keep the 128KB size for those performance improvements we can actually reproduced. |
|
It's more about the readability here, if the performance isn't worse |
Co-authored-by: Tony <68118705+Legend-Master@users.noreply.github.com>
|
Thanks! And also for the patience! |
On macOS, Wry now avoids an extra copy for owned custom protocol
response bodies of 128KB or larger by transferring the Vec directly into NSData.